Maybe this optimization isn't worth it and we should always set the PassiveSubtreeTag on the parent when a child is deleted. i.e. add Deletion to PassiveMask.

Effects are very common — any large tree is almost certain to have at least one. So the optimization really only kicks in when you delete a leaf node. Not sure it's worth the extra code.

I'm almost tempted to say that PassiveStatic isn't worth it for the same reason; but since we're planning to have other static tags for things like portals, PassiveStatic doesn't add any net additional overhead, so might as well keep it.

What do you think?

Maybe this optimization isn't worth it and we should always set the PassiveSubtreeTag on the parent when a child is deleted.

Given that the concept of static flags seems like a positive– and so keeping PassiveStatic around doesn't add any net overhead– the only question seems to be whether the few extra bytes required for us to check effectTag and subtreeTag are worth it, yeah?

With the current check, we might over traverse in the event that a child was deleted that had a passive effect, but no cleanup function. (Seems unlikely.) If we removed this check, we might over traverse any time a leaf node is deleted, period. Still, I guess that wouldn't be that bad since we'd only traverse the path to where the leaf was deleted.

TBH I think the cost is pretty small on either side (number or bytes or potential over traversing). Happy to remove the check for now.

cba61f7

If we removed this check, we might over traverse any time a leaf node is deleted, period. Still, I guess that wouldn't be that bad since we'd only traverse the path to where the leaf was deleted.

Yeah I think that's fine. It's also related to the detachFiber discussion. Since we're moving most of that work to the passive phase, then we need to visit every deletion during the passive phase anyway.

That's a good point I guess. I hadn't considered that aspect.

I consider the "detach" methods as kind of an optional optimization.

If a Deletion always gets a passive effect scheduled on it, as suggested above, we could move this whole function to the passive phase. (Except maybe return and alternate, since those affect semantics, like if you call setState on an unmounted component.)

If we don't clear return during layout, it would break a few things:

• The unmounted state warning
• How events bubble for the new API Dominic has been working on

That being said, it would fix a limitation of the Profiler API when it comes to measuring time spent in passive effect cleanup. That time currently doesn't get reported during an unmount (either a regular unmount or because of an error boundary) because the return pointer has been detached previously and so it can't bubble up to the nearest Profiler node. If we deferred detaching it until after passive, that time would be reported which is a plus.

Not sure if there would be a viable workaround for the event case, but for the unmounted state warning- we could walk the return path and look for Fibers with Deletion effects scheduled, except that this would have false positives for deletions that were scheduled and not committed so that's a bummer.

I guess we could always set another bit to mark a Fiber as having been deleted but at that point we aren't really saving much (aside from the Profiler fix).

That being said, I believe we can safely defer everything except the return pointer cleanup to the passive effects phase which would be slightly better.

7d618ed